Skip to content

Widget fix #11412

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Apr 21, 2025
Merged

Widget fix #11412

merged 2 commits into from
Apr 21, 2025

Conversation

qqmyers
Copy link
Member

@qqmyers qqmyers commented Apr 8, 2025

What this PR does / why we need it: This PR fixes an issue and adds several improvements:

  • It addresses an issue with the original script where, as far as we can tell, it could be delayed in running until the page had additional loaded scripts and the original logic assuming that widgets.js was the last script on the page was faulty. The script now waits for the page to load and will either find the script by id (new) or will scan all scripts to find the one with widgets.js in the source (legacy support).
  • It updates outdated document.write calls with ones that append the iframe/text/scripts below the original script tag
  • It updates jquery to 3.6.0 as used in Dataverse (although jquery is not used in this script, it pre-loads it for widgets-host.js)
  • Various cleanup/reorg

Which issue(s) this PR closes:

  • Closes #

Special notes for your reviewer:

Suggestions on how to test this: Follow the instructions on the collection and dataset edit/theme and widgets panes to add a widget to some other website (I had good luck just using local html files, but any web page should work) and verify that the Dataverse content shows. For Harvard Dataverse, should be tested with the current challenges to verify that it either works or shows the "Max challenge attempts exceeded. Please refresh the page to try again!" message and doesn't silently fail.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

Is there a release notes update needed for this change?:

Additional documentation:

@qqmyers qqmyers moved this to Ready for Triage in IQSS Dataverse Project Apr 8, 2025
@qqmyers qqmyers added the Size: 3 A percentage of a sprint. 2.1 hours. label Apr 8, 2025
@ofahimIQSS ofahimIQSS moved this from Ready for Triage to Ready for Review ⏩ in IQSS Dataverse Project Apr 15, 2025
@landreev landreev self-assigned this Apr 15, 2025
@landreev landreev self-requested a review April 15, 2025 15:30
@scolapasta scolapasta moved this from Ready for Review ⏩ to In Review 🔎 in IQSS Dataverse Project Apr 16, 2025
@landreev
Copy link
Contributor

landreev commented Apr 17, 2025 •
edited
Loading

I didn't mean to marinate this pr in "in review". I'm going to approve it, and we should merge it and take advantage of all the improvements listed.
I'm struggling with the fact the dataverse page still would not load under silent challenges, unless the browser already has a valid challenge session from another window. This cannot be addressed from inside this javascript, I'm pretty positive, meaning that it's outside of the scope of this pr. (And the javascript is now working consistently with a valid challenge session, which was not the case previously).
I was able to take it one step further:
With the current recommended setup, if the browser has not yet answered the challenge in another window, it fails to retrieve the widgets.js itself (for whatever reason), and then nothing happens. But if I put the javascript fragment itself someplace challenge-free, the browsers gets it and actually tries to load the page, and then fails with the "Max challenge attempts". I.e., in this case it does actually retrieve the challenge.js javascript and makes an honest effort to respond to it. ... Which makes me think that there may in fact be a way to figure out why it's failing, and address it, by tweaking the behavior of the page on the dataverse end.

Screenshot 2025-04-17 at 4 37 02 PM 
<?xml version="1.0" encoding="UTF-8"?><!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" lang="en">
  <head>
    widgets test
  </head>
  <body>
    <h2>Widgets Test</h2>

    <script id="dataverse-widget-js" src="https://dataverse.harvard.edu/api/resources/js/widgets.js?alias=king&amp;dvUrl=https://dataverse.harvard.edu&amp;widgetScope=king&amp;widget=iframe&amp;heightPx=500"></script>
  </body>
</html>

@github-project-automation github-project-automation bot moved this from In Review 🔎 to Ready for QA ⏩ in IQSS Dataverse Project Apr 17, 2025
@qqmyers
Copy link
Member Author

qqmyers commented Apr 17, 2025

FWIW: AWS does have Javascript you can add to respond to challenges triggered by fetches that your JavaScript does. There may be a little bit of chicken and egg if we don't want to require pasting their code into the host page, but I think that could be avoided by hosting widget.js itself elsewhere. If it isn't possible to just tweak the WAF challenge rules to avoid the issue we could look into this.

@ofahimIQSS ofahimIQSS self-assigned this Apr 18, 2025
@ofahimIQSS ofahimIQSS moved this from Ready for QA ⏩ to QA ✅ in IQSS Dataverse Project Apr 18, 2025
@landreev
Copy link
Contributor

The widgets.js itself is not a problem. We could put it on a different server off course. But in my experiments I just put it under an exempt location in prod. (as in the static html I posted)

... If it isn't possible to just tweak the WAF challenge rules

Loading the contents of the widget is a GET on dataverse.xhtml, our most crawled and abused page. I've been assuming there is no safe way of tweaking the rules in this case. Did you have a specific solution in mind? We could set up a "secret" alias for the page reserved for the widgets use, but that would only be "safe until discovered".

I'll be happy to experiment with the javascript aws provides. But I'm not sure it's going to help us here. Since the browser is already trying, but failing to respond to the challenge. (will look into this)
My uneducated working theory is that this "Max challenge attempts exceeded" condition is caused by extra server-side redirects. So I'm going to try and see if there are some redirects that the page makes when in widget mode (?) that can be eliminated.

This is not at the top of my stack r/n, but I want to find a solution eventually.

@qqmyers
Copy link
Member Author

qqmyers commented Apr 18, 2025

FWIW: I was thinking of this: https://docs.aws.amazon.com/waf/latest/developerguide/waf-js-challenge-api.html Seems like a way to pass the challenge before other page activity tries to access the protected site. Not sure it covers iframe loads, but it does look like it could cover widget.js doing a get of dataverse.xhtml and then writing into the iframe - or something. I haven't done any experimentation.

@landreev
Copy link
Contributor

Cool, I'll experiment with that.

@ofahimIQSS
Copy link
Contributor

Looks good

image
image

@ofahimIQSS ofahimIQSS merged commit 057c3ee into IQSS:develop Apr 21, 2025
8 checks passed
@github-project-automation github-project-automation bot moved this from QA ✅ to Merged 🚀 in IQSS Dataverse Project Apr 21, 2025
@ofahimIQSS ofahimIQSS removed their assignment Apr 21, 2025
@scolapasta scolapasta moved this from Merged 🚀 to Done 🧹 in IQSS Dataverse Project Apr 22, 2025
@landreev landreev assigned ofahimIQSS and unassigned landreev Apr 22, 2025
@landreev
Copy link
Contributor

FWIW, I've tried to experiment with the AWS-recommended approach.
Set up a static page with the
<script type="text/javascript" src="...our aws challenge url.../challenge.js” defer></script>
in the <head> section.
it looks like I can now insert const token = AwsWafIntegration.getToken(); into widgets.js; but I haven't been able to get it to send the obtained result as the x-aws-waf-token cookie when populating the iframe... So, I guess I do need to rewrite that js to explicitly use the fetch api to read dataverse.xhtml.

@pdurbin pdurbin added this to the 6.7 milestone May 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@landreev landreev landreev approved these changes

Assignees

@ofahimIQSS ofahimIQSS

Labels
Size: 3 A percentage of a sprint. 2.1 hours.
Projects
IQSS Dataverse Project
Status: Done 🧹
Milestone
6.7
Development

Successfully merging this pull request may close these issues.
4 participants
@qqmyers @landreev @ofahimIQSS @pdurbin